Skip to content

fix: consensus timeouts #6203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 25 commits into
base: main
Choose a base branch
from
Draft

fix: consensus timeouts #6203

wants to merge 25 commits into from
+58 −110

Conversation

@rootulp
Copy link
Collaborator

@rootulp rootulp commented Nov 14, 2025 •
edited
Loading

Closes #6178

Blocked on celestiaorg/celestia-core#2646

Testing

Locally on a single-node upgrade script

# Before v6 upgrade height

celestia-core rp/fix-proto-break $ curl http://localhost:26657/abci_info | jq .
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   443  100   443    0     0   432k      0 --:--:-- --:--:-- --:--:--  432k
{
  "jsonrpc": "2.0",
  "id": -1,
  "result": {
    "response": {
      "data": "celestia-app",
      "version": "5.0.12",
      "app_version": "5",
      "last_block_height": "2",
      "last_block_app_hash": "1+AfWN1MXufUHhKpjnPtMinL6Qq95LV4j4hx+J9Tw58=",
      "timeout_info": {
        "timeout_propose": "3500000000",
        "timeout_commit": "4200000000",
        "timeout_prevote": "0",
        "timeout_prevote_delta": "0",
        "timeout_precommit": "0",
        "timeout_precommit_delta": "0",
        "timeout_propose_delta": "0",
        "delayed_precommit_timeout": "0"
      }
    }
  }
}

# After v6 upgrade height
celestia-core rp/fix-proto-break $ curl http://localhost:26657/abci_info | jq .
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   505  100   505    0     0   769k      0 --:--:-- --:--:-- --:--:--  493k
{
  "jsonrpc": "2.0",
  "id": -1,
  "result": {
    "response": {
      "data": "celestia-app",
      "version": "6.2.5-20-g4cb16736a",
      "app_version": "6",
      "last_block_height": "14",
      "last_block_app_hash": "i0rn4LCuq9q3DweU79+sf717Tcz2Tl+xdedkO0e35Kk=",
      "timeout_info": {
        "timeout_propose": "8500000000",
        "timeout_commit": "1000000",
        "timeout_prevote": "3000000000",
        "timeout_prevote_delta": "500000000",
        "timeout_precommit": "3000000000",
        "timeout_precommit_delta": "500000000",
        "timeout_propose_delta": "500000000",
        "delayed_precommit_timeout": "5850000000"
      }
    }
  }
}

@rootulp rootulp self-assigned this Nov 14, 2025
@rootulp
Copy link
Collaborator Author

rootulp commented Nov 14, 2025

Reproduced the issue.

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 15, 2025 

        	Error:      	Not equal:
        	            	expected: 1s
        	            	actual  : 4.2s
        	Test:       	TestCelestiaTestSuite/TestCelestiaAppV5ToV6
        	Messages:   	v5 timeout_propose_delta mismatch

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 15, 2025 •
edited
Loading

On a v5.x binary I get the expected behavior:

celestia-app v5.x $ curl http://localhost:26657/abci_info | jq .
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   279  100   279    0     0   362k      0 --:--:-- --:--:-- --:--:--  272k
{
  "jsonrpc": "2.0",
  "id": -1,
  "result": {
    "response": {
      "data": "celestia-app",
      "version": "5.0.12",
      "app_version": "5",
      "last_block_height": "4",
      "last_block_app_hash": "8KbLhafGNEyuTNLRcOL4a1Ahx4ltRtH7fsdx7RAJGO0=",
      "timeout_info": {
        "timeout_propose": "3500000000",
        "timeout_commit": "1000000000"
      }
    }
  }
}

On a v6 binary (with multiplexer) with app version 5 I get unexpected behavior:

celestia-app rp/fix-consensus-timeouts $ curl http://localhost:26657/abci_info | jq .
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   445  100   445    0     0   460k      0 --:--:-- --:--:-- --:--:--  434k
{
  "jsonrpc": "2.0",
  "id": -1,
  "result": {
    "response": {
      "data": "celestia-app",
      "version": "5.0.12",
      "app_version": "5",
      "last_block_height": "444",
      "last_block_app_hash": "gNmIq46ir5gz5TO1BH1oGopzOK2XGQn6tL/SrumJwMw=",
      "timeout_info": {
        "timeout_propose": "3500000000",
        "timeout_propose_delta": "1000000000",
        "timeout_prevote": "0",
        "timeout_prevote_delta": "0",
        "timeout_precommit": "0",
        "timeout_precommit_delta": "0",
        "timeout_commit": "0",
        "delayed_precommit_timeout": "0"
      }
    }
  }
}

The timeout_commit is wrong. Also timeout_propose_delta should likely also be 0.

@rootulp rootulp mentioned this pull request Nov 15, 2025
Open
@rootulp
Copy link
Collaborator Author

rootulp commented Nov 16, 2025 •
edited
Loading

Had to use a modified v5.x binary to get helpful debug logs

v5.x app/app.go Info result: timeout_info:<timeout_propose:<seconds:3 nanos:500000000 > timeout_commit:<seconds:4 nanos:200000000 > >
[DEBUG] multiplexer/abci/remote_v2.go Info resp: data: timeout_info:<timeout_propose:<seconds:3 nanos:500000000 > timeout_propose_delta:<seconds:4 nanos:200000000 > >

There's a bug! We're converting from timeout_commit -> timeout_propose_delta!

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 16, 2025

We made a breaking change to protos 🤯

https://github.com/celestiaorg/celestia-core/blob/c3c13760ca55a06830e03a582d362e53e1724229/proto/tendermint/abci/types.proto#L534-L537

https://github.com/celestiaorg/celestia-core/blob/1dea4a99c84084ab11089466e2db1b0bcc46aa20/proto/tendermint/abci/types.proto#L534-L543

@rootulp rootulp mentioned this pull request Nov 16, 2025
Open
@rootulp
Copy link
Collaborator Author

rootulp commented Nov 16, 2025

Ready for review. Can't be merged though

@rootulp rootulp marked this pull request as ready for review November 16, 2025 21:13
@rootulp rootulp mentioned this pull request Nov 16, 2025
Draft
@rootulp rootulp marked this pull request as draft November 17, 2025 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evan-forbes evan-forbes evan-forbes approved these changes

@rach-id rach-id Awaiting requested review from rach-id rach-id is a code owner

@ninabarbakadze ninabarbakadze Awaiting requested review from ninabarbakadze ninabarbakadze is a code owner

@cmwaters cmwaters Awaiting requested review from cmwaters cmwaters is a code owner

Assignees

@rootulp rootulp

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consensus timeouts are not properly set

2 participants

@rootulp @evan-forbes